Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass metadata to gotenberg-chromium processor #66

Merged

Conversation

muratbinerbay
Copy link
Contributor

@muratbinerbay muratbinerbay commented Mar 28, 2024

This will let us pass metadata to the PDF file like title, author, copyright, and creation date.

Documentation links:

Copy link

github-actions bot commented Mar 28, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@muratbinerbay
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@muratbinerbay
Copy link
Contributor Author

recheck

@kingjia90 kingjia90 added this to the 1.3.0 milestone Mar 29, 2024
@kingjia90 kingjia90 self-assigned this Mar 29, 2024
@muratbinerbay muratbinerbay force-pushed the pass-metadata-to-gotenberg-chromium branch from ab86471 to 0d5a0b1 Compare April 2, 2024 08:34
@muratbinerbay muratbinerbay changed the base branch from 1.2 to 1.x April 2, 2024 08:34
@muratbinerbay muratbinerbay requested a review from kingjia90 April 2, 2024 08:48
src/Processor/Gotenberg.php Outdated Show resolved Hide resolved
@muratbinerbay
Copy link
Contributor Author

muratbinerbay commented Apr 2, 2024

@kingjia90 on v2.2.0 the formValue has protected access. I reverted last 2 commits to what @blankse suggested.

https://github.com/gotenberg/gotenberg-php/blob/main/src/MultipartFormDataModule.php#L92

@blankse
Copy link
Contributor

blankse commented Apr 2, 2024

@muratbinerbay @kingjia90 The formValue is only in 1.1.8 public.
See:
gotenberg/gotenberg-php@33298eb
gotenberg/gotenberg-php@993384d
So we need to bump the version in composer, right?
"gotenberg/gotenberg-php": "^1.1.8 || ^2.2",

@kingjia90
Copy link
Contributor

So many plot twists 😄 Thank you for the changes.
It seems that PHPStan is triggering some false positive i guess, do we need to mute these?

@blankse
Copy link
Contributor

blankse commented Apr 2, 2024

@kingjia90 Yes PhpStan doesn't want the formValue method because it is protected in 2.2. But it isn't reachable there.

src/Processor/Gotenberg.php Outdated Show resolved Hide resolved
@muratbinerbay
Copy link
Contributor Author

@blankse @kingjia90 do we want to try something like ReflectionMethod. Seems rather excessive to me but I'm willing to try it :)

@blankse
Copy link
Contributor

blankse commented Apr 2, 2024

Doesn't work :(

I think we need the same construct like in pimcore/pimcore. We have there a own phpstan config for lowest with reportUnmatchedIgnoredErrors: false:
https://github.com/pimcore/pimcore/blob/11.x/phpstan-lowest.neon

So we can create a phpstan baseline with the highest dependencies.

@blankse
Copy link
Contributor

blankse commented Apr 2, 2024

Of course, we could also process the metadata with version 2.2 only, if that is an alternative :)

@muratbinerbay
Copy link
Contributor Author

Of course, we could also process the metadata with version 2.2 only, if that is an alternative :)

would also be acceptable in my opinion, @kingjia90 what do you think?

@kingjia90
Copy link
Contributor

kingjia90 commented Apr 2, 2024

That would be also fine to me, no any objection, but it seems that then it would need a doc or changelog entry (as example) to better explain that "difference" (that metadata is available only if you use gotenberg-php 2.2+ )

maybe let's have a final confirmation by @fashxp first on how to proceed

@muratbinerbay
Copy link
Contributor Author

Hi @kingjia90 @fashxp, just wanted to ping you to check if I can go ahead and make the metadata available only for gotenberg-php:2.2+ ?
Thanks!

@kingjia90
Copy link
Contributor

kingjia90 commented May 16, 2024

Thank you for your patience @muratbinerbay !

I think it would be fine to support metadata only for 2.2+ and we can proceed with that, we also have a similar situation with the screenshot feature in 2.0 and it's "diverging" more and more in what one version can do and what not, so at some point we just need to properly document the difference and recommend to use the latest when possible.

@muratbinerbay muratbinerbay requested a review from kingjia90 May 30, 2024 11:45
@kingjia90 kingjia90 merged commit 2c3934b into pimcore:1.x Jun 7, 2024
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2024
@muratbinerbay muratbinerbay deleted the pass-metadata-to-gotenberg-chromium branch June 7, 2024 13:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants